Skip to content

chore: re-writing ./web using RSPack and tailwind #591

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 77 commits into from
Sep 3, 2024
Merged

chore: re-writing ./web using RSPack and tailwind #591

merged 77 commits into from
Sep 3, 2024

Conversation

ZibanPirate
Copy link
Member

@ZibanPirate ZibanPirate commented Jul 9, 2024

web:

  • removed pages: articles, and learn, their data is still available.
  • replaced webpack with rsbuild.
  • replaced @dzcode.io/ui with tailwind (using daisyui).

mobile:

  • excluded ./mobile from mono repo (from both npm and lerna)
  • removed mobile section of the landing page until we bring mobile back (i'm experimenting with crux currently).

api:

  • deployed api to new VPS both stage and prod
  • removed extra info (fetched from github) from the projects endpoint.

Type of change

  • Big Refactor

@ZibanPirate ZibanPirate self-assigned this Jul 9, 2024
}

const routes: RouteInterface[] = [
let routes: Array<
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let -> const

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we are mutating few lines down

Copy link
Member

@omdxp omdxp Sep 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still in javascript an array can be mutable even if it is const, unless you force the TS compiler with as const at the end so it will not throw a compilation error.

I'd also argue why should we mutate the path? why not having just on immutable path per each page?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i see your point, i guess i could loop thorough it using forEach instead of .map, i will fix it in another PR ...

it's down to code styling, i wouldn't be too strict on that

</Helmet>
<Stack direction="vertical" min={{ height: "100vh" }}>
<Navbar
<div className="flex flex-col min-h-screen">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another reason why I don't like tailwind

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?? elaborate cuz i don't see an issue here

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can foresee even longer class names in the future (even if it does have a cool vs code extension for autocompletion)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These values can be stored as json files.

Why? because it can be useful to leverage other platforms like Localise to handle translation updates

Copy link
Member

@omdxp omdxp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's go 🚀

PS: we can address later those conversations

@ZibanPirate ZibanPirate merged commit d557d10 into main Sep 3, 2024
8 checks passed
@ZibanPirate ZibanPirate deleted the rspack branch September 3, 2024 21:38
@ZibanPirate ZibanPirate added this to the Stable Infrastructure milestone Sep 4, 2024
@ZibanPirate ZibanPirate added enhancement New feature or request breaking-change Breaking Changes labels Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Breaking Changes enhancement New feature or request
Projects
Status: Merged
Development

Successfully merging this pull request may close these issues.

3 participants